Fully automatic review from Claude Code

Review of PR #12334

Root Cause Analysis — Verified

I traced the full execution path. The PR description is accurate.

DefaultLifecycleRegistry.CleanLifecycle (line 429) and SiteLifecycle (line 547) call Lifecycles.plugin(...) to create lifecycle bindings. Before this PR, the PluginExecution was built without setting priority, defaulting to 0 — the same as user-defined executions from pom.xml.

DefaultLifecycleMappingDelegate (line 99) sorts executions within a phase using a TreeMap<PhaseId, ...> with PhaseComparator, which sorts by Integer.compare(p1.priority(), p2.priority()) (line 65) — lower priority number first. When priorities are tied (both 0), ordering falls to plugin iteration order.

LifecycleBindingsMerger.mergePluginContainer_Plugins inserts pom.xml plugins into the LinkedHashMap first (as target), lifecycle plugins second (as source). So at equal priority, pom.xml executions come first — causing the regression.

For comparison, DefaultPackagingRegistry (line 155) sets priority(i - mojos.size()) for default lifecycle packaging bindings, always producing negative values. This is why the default lifecycle was unaffected.

The Fix — Correct

Adding .priority(-1) makes clean/site bindings sort before user executions (priority 0) in both execution paths:

1. Sequential path: DefaultLifecycleMappingDelegate → PhaseComparator → Integer.compare(-1, 0) → lifecycle binding first
2. Concurrent path: BuildPlanExecutor (line 600) → BuildStep.addMojo(mojo, priority) → TreeMap<Integer, ...> key ordering → lifecycle binding first

The LifecycleBindingsMerger.mergePluginExecution_Priority also handles this correctly — it keeps min(target, source), so the -1 from lifecycle bindings is preserved during model merging. The compat layer (DefaultLifecycleBindingsInjector, line 154) uses Math.min() — same correct behavior.

Observations

The value -1 is consistent. DefaultPackagingRegistry uses i - mojos.size() which for a single mojo (i=0, size=1) produces exactly -1. Since Lifecycles.plugin() always creates a single execution, hardcoding -1 is equivalent.

No test for the fixed behavior. Existing tests pass and manual reproduction confirms the fix, but there's no new test that asserts lifecycle bindings execute before pom.xml executions in the clean/site lifecycle. This is the kind of regression that could easily reoccur if someone refactors lifecycle binding creation. A unit test or integration test verifying the execution order (like the manual reproducer from #12331) would guard against that.

The PR description is excellent — clear root cause, comparison with the working default lifecycle path, and a concise fix section.

Fully automatic review from Claude Code

Fix is correct, minimal, and now has test coverage. Approving.

The .priority(-1) value is consistent with DefaultPackagingRegistry (which produces -1 for single-mojo bindings via i - mojos.size()), both execution paths (sequential via PhaseComparator and concurrent via BuildStep.mojos TreeMap) sort correctly, and the merger preserves the lower priority during model merging.